tests/cross_validation/test_tukey_window.py (2)

23-24: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Add type annotations to the test function.

Adding type hints for the parameter and return type improves static analysis and maintainability.

💡 Proposed refactor

 `@pytest.mark.parametrize`("duration", [4, 128])
-def test_default_tukey_window_matches_bilby(duration):
+def test_default_tukey_window_matches_bilby(duration: int) -> None:

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/cross_validation/test_tukey_window.py` around lines 23 - 24, Add a type
annotation for the duration parameter and annotate
test_default_tukey_window_matches_bilby with its return type, preserving the
existing parametrization and test behavior.

---

7-20: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Simplify conditional dependency skipping with Pytest.

The manual try/except block and pytestmark can be replaced with pytest.importorskip(), which is Pytest's idiomatic way to handle missing test dependencies. It natively skips the module and can also retain the helpful installation instruction that was previously being swallowed by the except block.

♻️ Proposed refactor

-from tests.utils import check_bilby_available
-
-try:
-    check_bilby_available()
-    import bilby
-
-    BILBY_AVAILABLE = True
-except ImportError:
-    BILBY_AVAILABLE = False
-
-pytestmark = pytest.mark.skipif(
-    not BILBY_AVAILABLE,
-    reason="bilby required for cross-validation tests",
-)
+pytest.importorskip(
+    "bilby",
+    reason="cross_validation tests require bilby. Install with: pip install bilby",
+)
+import bilby

(Note: Explicit import bilby is preserved after the skip check to ensure static type checkers like Pyright can still correctly resolve the module).

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/cross_validation/test_tukey_window.py` around lines 7 - 20, Replace the
manual bilby availability check and module-level pytestmark with
pytest.importorskip(), preserving the check_bilby_available() installation
guidance before the skip and retaining the explicit bilby import afterward for
static type checking.

src/jimgw/core/single_event/data.py (2)

254-279: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Document the new RuntimeError in the docstring.

fft() can now raise RuntimeError once frequency-domain data are fixed (line 265-267), but the docstring has no Raises: section, unlike the sibling set_tukey_window() which documents its exceptions thoroughly.

📝 Proposed fix

         Args:
             window: Window function to apply to the data before FFT (default: None).
+
+        Raises:
+            RuntimeError: If a custom window is supplied after frequency-domain
+                data have already been fixed.
         """

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/jimgw/core/single_event/data.py` around lines 254 - 279, Update the fft
method docstring to add a Raises section documenting the RuntimeError raised
when a window is supplied after frequency-domain data has been fixed. Keep the
existing argument and behavior documentation unchanged.

---

144-189: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Add a shape check for an explicit window.

delta_t, alpha, and roll_off all now get explicit range/finite validation, but a caller-supplied window (line 189, self.window = window) is never checked against td's length. A mismatched shape will only surface later as a cryptic broadcasting error inside fft() (self.td * window), instead of a clear message at construction time.

🛡️ Proposed fix

         if window is None:
             if self.is_empty:
                 # Empty Data instances are used as detector placeholders.
                 self.window = jnp.array([])
             else:
                 self.set_tukey_window()
         else:
+            if window.shape != self.td.shape:
+                raise ValueError(
+                    "window must have the same shape as td"
+                )
             self.window = window

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/jimgw/core/single_event/data.py` around lines 144 - 189, Validate an
explicitly supplied window in the __init__ method before assigning self.window:
for non-empty data, ensure its shape matches td’s shape or length and raise a
clear ValueError on mismatch. Preserve the existing empty-data behavior and
automatic Tukey-window initialization when window is None.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes